Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore(ux): improve error output for unknown spec flag #1221

Merged
merged 19 commits into from
Dec 29, 2023
Merged

chore(ux): improve error output for unknown spec flag #1221

merged 19 commits into from
Dec 29, 2023

Conversation

qweeah
Copy link
Contributor

@qweeah qweeah commented Dec 27, 2023 •
edited
Loading

What this PR does / why we need it:
This PR improves error output of invalid spec option and fixes #1217

  • The parsing of --image-spec and --distribution-spec were refactored into native pflag parsing
  • changed the output of invalid value for both flags to 
    $ oras attach --distribution-spec 123
Error: invalid argument "123" for "--distribution-spec" flag: unknown distribution 
specification flag: 123
Available options: v1.1-referrers-tag, v1.1-referrers-api
$ oras push --image-spec 123      
Error: invalid argument "123" for "--image-spec" flag: unknown image specification flag: 123
Available options: v1.1, v1.0

Please check the following list:

  • Does the affected code have corresponding tests, e.g. unit test, E2E test?
  • Does this change require a documentation update?
  • Does this introduce breaking changes that would require an announcement or bumping the major version?
  • Do all new files have an appropriate license header?

@qweeah
chore(ux): improve error output for unknown spec flag
dbe605b 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
fix e2e
a77ff5c 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
add e2e tests adnd default value
a64c84d 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
remove focus spec
649619f 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
fix e2e
830ecc9 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah qweeah marked this pull request as draft December 27, 2023 08:46
@qweeah
fix e2e
8283f8f 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
fix e2e
41913d1 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
fix e2e
83d6843 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@codecov-commenter
Copy link

codecov-commenter commented Dec 27, 2023 •
edited by codecov bot
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c1a3185) 79.89% compared to head (c12f05c) 80.10%.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1221      +/-   ##
==========================================
+ Coverage   79.89%   80.10%   +0.21%     
==========================================
  Files          67       67              
  Lines        3695     3719      +24     
==========================================
+ Hits         2952     2979      +27     
+ Misses        524      522       -2     
+ Partials      219      218       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@qweeah qweeah marked this pull request as ready for review December 27, 2023 12:37
shizhMSFT
shizhMSFT reviewed Dec 28, 2023
View reviewed changes
cmd/oras/internal/option/spec.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/spec.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/spec.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/spec.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/spec.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/spec.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/spec.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/spec.go Outdated Show resolved Hide resolved
@qweeah
resolve comment
2d4f383 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
resolve comment
35e960e 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
fix e2e
645e3ad 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
fix e2e
2c1ca82 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
resolve comments
2e4c097 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
fix e2e
db738cc 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah qweeah marked this pull request as draft December 28, 2023 08:14
@qweeah qweeah marked this pull request as ready for review December 28, 2023 08:15
shizhMSFT
shizhMSFT reviewed Dec 29, 2023
View reviewed changes
cmd/oras/internal/option/spec.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/spec.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/spec.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/spec.go Outdated Show resolved Hide resolved
@qweeah
resolve comments
8dd8514 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
increase coverage
b6341e9 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
fix e2e
c12f05c 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
shizhMSFT
shizhMSFT approved these changes Dec 29, 2023
View reviewed changes
Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@qweeah qweeah merged commit f8be882 into oras-project:main Dec 29, 2023
8 checks passed
@qweeah qweeah mentioned this pull request Jan 19, 2024
Closed
@qweeah qweeah mentioned this pull request Feb 1, 2024
Merged
FeynmanZhou pushed a commit to FeynmanZhou/oras that referenced this pull request May 11, 2024
@qweeah @FeynmanZhou
chore(ux): improve error output for unknown spec flag (oras-project#1221
30a9710 
)

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Feynman Zhou <feynmanzhou@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shizhMSFT shizhMSFT shizhMSFT approved these changes

@sajayantony sajayantony Awaiting requested review from sajayantony sajayantony is a code owner

@SteveLasker SteveLasker Awaiting requested review from SteveLasker SteveLasker is a code owner

@TerryHowe TerryHowe Awaiting requested review from TerryHowe TerryHowe is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

improve error message when invalid options given when pushing
3 participants
@qweeah @codecov-commenter @shizhMSFT